-
Notifications
You must be signed in to change notification settings - Fork 39
fix: add APT dependencies to some scripts for scalingo-24-minimal #529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
eb95b19 to
14649eb
Compare
m4 is a dependency4fe814e to
ba5bc52
Compare
84286fa to
c542e43
Compare
| fi | ||
|
|
||
| php_version="$1" | ||
| php_series="$(echo $php_version | cut -d '.' -f1,2)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable is never used
| export C_INCLUDE_PATH="$C_INCLUDE_PATH:/app/vendor/libonig/include" | ||
| export PKG_CONFIG_PATH="/app/vendor/libonig/lib/pkgconfig:$PKG_CONFIG_PATH" | ||
|
|
||
| echo "-----> Downloading dependency WebP library ${webp_version}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just moved this earlier
| echo "-----> Downloading dependency zlib ${zlib_version}" | ||
| curl -LO "${PHP_BASE_URL}/zlib/zlib-${zlib_version}.tar.gz" | ||
| tar -xzvf "zlib-${zlib_version}.tar.gz" | ||
| tar -xzf "zlib-${zlib_version}.tar.gz" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the verbosity of the tar calls as it pollutes stdout.
| ) | ||
|
|
||
| WITH_CURL_CONFIGURE="" | ||
| WITH_CURL_CONFIGURE="--with-curl=/usr" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This default value was already in the call to configure. We had a duplicated with-curl so I'm fixing this.
| export WEBP_LIBS="-L/app/vendor/libwebp/lib -lwebp" | ||
| export WEBP_CFLAGS="-I/app/vendor/libwebp/include" | ||
|
|
||
| ENABLE_GD_CONFIGURE="--enable-gd --with-jpeg --with-freetype --with-webp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these options in the configure call directly, I find this clearer
| tar -xzf "zlib-${zlib_version}.tar.gz" | ||
|
|
||
| echo "-----> Downloading dependency libonig ${libonig_version}" | ||
| libzip_flag="" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This always empty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I took the opportunity to slightly improve the
package_phpscript.Related to STORY-2492